Skip to content

feat: one-key client for the new private key - #137

Open
sydneycollins-cio wants to merge 4 commits into
mainfrom
new-api-keys
Open

sydneycollins-cio wants to merge 4 commits into
mainfrom
new-api-keys

Conversation

@sydneycollins-cio

Copy link
Copy Markdown

Update to support the new private key

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sydneycollins-cio

Copy link
Copy Markdown
Author

Tested locally at this PR's head (851a0b71cd) against services 199c4a789c, CDP aa721dbec and edge af983b44, all at main. Keys were made through services' UI API and synced to CDP and edge the normal way. A pass-through proxy recorded the auth scheme and status. CDP ran in strict mode, so a bad key gets an error instead of 200.

Key Call Result
ak_ identify Bearer, 200
ak_ track event Bearer, 200
ak_ send email Bearer, 200 on /v1/send/email
wk_ identify Bearer, 200
wk_ send email refused by the SDK (no transactional access), no request sent
legacy App API key send email Bearer, 200
legacy site ID + API key old Track client identify Basic, 200
revoked ak_ identify 401, SDK reports the key can't use Track

No key showed up in the SDK's output or logs. All passed.

@sydneycollins-cio
sydneycollins-cio marked this pull request as ready for review October 13, 2026 16:35

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 851a0b7. Configure here.

Comment thread customerio/client.py Outdated

@Shahroz16 Shahroz16 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a .netrc case in _BearerTrackClient: setting session.auth = None lets Requests replace the supplied Bearer header with Basic auth if there's a matching entry for the API host. Could we use an explicit Bearer auth handler and cover that with a prepared-request check? It only happens with that matching entry, but we'd still be sending different credentials from the ones the caller passed.

Two smaller wording follow-ups: 401/403 can also mean an IP restriction or a disabled workspace, so “this key can't…” is a bit too specific. And the README should qualify legacy-key support: transactional calls work, but Bearer Track calls on Pipelines workspaces need a source-bound new key.

I'd like us to sort out the URL handling in the existing thread and this auth case before merging. That's my take. Let's also get Bernard or the NF team to review this.

sydneycollins-cio and others added 3 commits October 15, 2026 18:11
Read the region from any key shaped {xx}_{us|eu}_ and treat pk_ as the
public key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… errors

A matching .netrc entry could replace the Bearer header with Basic auth.
401/403 errors no longer blame the key, and the README says which calls
legacy keys still cover.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sydneycollins-cio

Copy link
Copy Markdown
Author

Addressed the review in 72295df:

  • The track client sends the key through a Bearer auth handler, so a matching .netrc entry no longer swaps in Basic auth. A test checks the prepared request keeps Bearer <key>.
  • 401/403 messages no longer say "this key can't". They say the request was refused and to check the key, permissions, IP allowlist and workspace status.
  • README notes that legacy keys still work for App API calls, but Pipelines tracking needs a new source-bound sk_ key.
  • The trailing-slash URL fix (c103316) is still in place.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants